Skip to content

Conversation

@rob100
Copy link
Contributor

@rob100 rob100 commented Aug 6, 2015

I started to fill the section about precompiled headers with some general information and some links to manuals for the common compiler. Any additional suggestions?

Filled the section about precompiled headers with some general
information and some links to a manual for the common compiler
@lefticus
Copy link
Member

lefticus commented Aug 6, 2015

I'd say two things:

  1. There should be some mention that if you are shipping a library of some sort that you need your build system to build both with and without PCH enabled so that you know if you have broken header requirements anywhere (since PCH essentially makes all of the common includes global to every single .cpp file in the project). This is something I've seen in the real world where a PCH build worked but a non-pch did not.
  2. It might be worth mentioning that your build system might have some support for PCH, like the cmake add-in cotire

@rob100
Copy link
Contributor Author

rob100 commented Aug 7, 2015

1.) That's a good point. I had some trouble with this in the past too. I will add something about the possible broken header requirements. This page summarizes some more drawbacks for precompiled headers usage in GCC: https://gcc.gnu.org/wiki/PCHHaters
Some of the points are valid for other compilers too.
2.) I did not know this tool, but it sounds interesting. Should definitely be worth mentioning.

@rob100
Copy link
Contributor Author

rob100 commented Aug 24, 2015

I added some more informations to the disadvantages of precompiled headers and noted cotire. As always feel free to improve my English.

lefticus added a commit that referenced this pull request Aug 24, 2015
@lefticus lefticus merged commit 2f5e538 into cpp-best-practices:master Aug 24, 2015
@rob100 rob100 deleted the precompiled_header branch August 25, 2015 09:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants